Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add installation behaviors to CMakeLists.txt #1317

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adamkewley
Copy link

@adamkewley adamkewley commented Jul 8, 2024

This isn't a PR you guys should necessarily merge, but just another datapoint on how to (potentially) handle installation packaging.

Background

Just for context, my use-case is that I'm experimenting with adding some sandboxed scripting to:

I really liked the vibe/design/portability of luau because I'm also in the middle of gradually porting the project's backend to wasm (WIP) and would eventually like users to be able to experiment with their own physics components etc. in a browser without a server-side backend and eventually provide a safe-to-distribute model format that includes scripted components.

Because I deploy to 3 (soon, 4) platforms, the way in which I handle dependencies is to provide a separate source-build of all of dependencies in third_party. The directory contains its own CMakeLists.txt, which uses ExternalProject_Add to build+install each dependency into a unified directory that can then be added to the CMAKE_PREFIX_PATH of some downstream project (for me, it's opensim-creator). The idea is that some dependencies can be disabled, layered over the OS-provided ones, etc. without having to have custom find_package steps in opensim-creator (i.e. it can all be handled from the top-level build bash script or similar).

Other PRs Like this

  • Added install commands to CMakeLists.txt #1166 , which isn't handy for me because you can't find_package in a downstream project
  • Add cmake install target #926, which I gracefully ripped off and changed a little bit based on personal preferences. Heavy credit should be given to Add cmake install target #926 . My (sometimes, taste-based) changes from it were:
    • I don't hard-code the installation dirs as much and try to fallback to cmake's defaults or ${CMAKE_INSTALL_LIBDIR} etc.
    • The install-related steps are closer to where the targets are configured, rather than being in a separate location, to minimize having to separately check if(BUILDING_CLI)-type flags
    • I don't version it

Testing I've Completed

I made these changes and now I can just find_package in my downstream branch. I'll report back once I have a branch building on all 4 (Win/MacOS/Linux/Emscripten) platforms.


target_include_directories(isocline PUBLIC extern/isocline/include)

target_include_directories(Luau.VM.Internals INTERFACE VM/src)
target_include_directories(Luau.VM.Internals INTERFACE
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I get that this is an internal thing, but cmake will cry about the fact that public targets that are being installed are dependent on a target that isn't.

@theoparis theoparis mentioned this pull request Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant